docs: functions documented with examples #226

antonengelhardt · 2024-02-27T16:25:37Z

In-Code Documentation for the following functions:

RootContext:

on_vm_start()
get_vm_configuration()
on_configure()
get_plugin_configuration()
set_tick_period()
on_tick()

Context:

dispatch_http_call()
on_http_call_response()
get_http_call_response_headers()
get_http_call_response_headers_bytes()
get_http_call_response_header()
get_http_call_response_header_bytes()
get_http_call_response_body()
get_http_call_response_trailers()
get_http_call_response_trailers_bytes()
get_http_call_response_trailer()
get_http_call_response_trailer_bytes()

HttpContext:

on_http_request_headers()
get_http_request_headers()
get_http_request_headers_bytes()
get_http_request_header()
get_http_request_header_bytes()
add_http_request_header()
send_http_response()

Inspired by proxy-wasm/spec#42

Why?

To have documentation in your editor:

google-cla · 2024-02-27T16:25:41Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

* `dispatch_http_call()` * `on_http_call_response()`` * `on_vm_start()` * `get_vm_configuration()` * `on_configure()` * `get_plugin_configuration()` * `set_tick_period()` * `on_tick()` * `on_http_request_headers()` * `get_http_request_headers()` * `get_http_request_header()` * `add_http_request_header()` * `resume_http_context()` * `send_http_response()` Signed-off-by: Anton Engelhardt <[email protected]>

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt · 2024-02-27T21:24:14Z

All examples do not include the RootContext implementation or main! Macro, as they are pretty similar in every case. So copy and pasting the examples, won't work, but I can still add them, if you think it's better.

antonengelhardt · 2024-04-18T20:06:04Z

@PiotrSikora What do you think? Sorry to ping you, but I think this would be a great benefit for DX

PiotrSikora

@PiotrSikora What do you think? Sorry to ping you, but I think this would be a great benefit for DX

I agree those would be very helpful. Sorry for the delay, I'll review this in the coming days.

src/traits.rs

… stuff Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt · 2024-04-30T15:07:45Z

@PiotrSikora What do you think? Sorry to ping you, but I think this would be a great benefit for DX

I agree those would be very helpful. Sorry for the delay, I'll review this in the coming days.

@PiotrSikora I tested all documented functions again and set up a repo with 7 folders that cover all functions. It might still be that something is missing. I will do some diffs on the docs and the testing-repo-files to see if anything is missing apart from the boilerplate stuff (proxy_wasm::main!, RootContext Impl, etc.).

I hope this makes the review as easy as possible.

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt · 2024-04-30T17:25:46Z

I can squash my changes into one commit, if required.

antonengelhardt · 2024-05-13T12:39:10Z

@PiotrSikora I can also add the examples from the mentioned repo to the examples/ directory...

Signed-off-by: Anton Engelhardt <[email protected]>

PiotrSikora · 2024-08-13T07:22:28Z

src/traits.rs

+    /// * `OK` on success.
+    /// * `BAD_ARGUMENT` for unknown upstream, or when headers are missing required `:authority`, `:method` and/or `:path` values.
+    /// * `INTERNAL_FAILURE' when the host failed to send requested HTTP call.
+    /// * `INVALID_MEMORY_ACCESS` when `upstream_data`, `upstream_size`, `headers_data`, `headers_size`, `body_data`, `body_size`, `trailers_data`, `trailers_size` and/or `return_call_id` point to invalid memory address.


It looks that you've copied those return values from the spec, which is generally fine... However, this SDK:

combines *_size and *_data into vectors and strings,

assumes that the memory objects are correct, so INVALID_MEMORY_ACCESS cannot happen and if it does, then it will result in panic,

it's opinionated about which return values are allowed, so if you want to document them, then take a look at src/hostcalls.rs and see which return values are there.

I removed the docs from the spec and wrote something myself. Is that better?

I didn't mean that including text from the (now merged) spec is wrong. I meant that it had to be adjusted to account for the higher-level types used in the SDK.

I'll take a look at it in the next few days, thanks!

src/traits.rs

PiotrSikora · 2024-08-13T07:29:03Z

@PiotrSikora I can also add the examples from the mentioned repo to the examples/ directory...

I took a look at them, and they seem to be mostly duplicates of the existing examples.

Signed-off-by: Anton Engelhardt <[email protected]>

.github/workflows/rust.yml

PiotrSikora · 2024-10-14T04:42:17Z

src/traits.rs

+    /// * `OK` on success.
+    /// * `BAD_ARGUMENT` for unknown upstream, or when headers are missing required `:authority`, `:method` and/or `:path` values.
+    /// * `INTERNAL_FAILURE' when the host failed to send requested HTTP call.
+    /// * `INVALID_MEMORY_ACCESS` when `upstream_data`, `upstream_size`, `headers_data`, `headers_size`, `body_data`, `body_size`, `trailers_data`, `trailers_size` and/or `return_call_id` point to invalid memory address.


I didn't mean that including text from the (now merged) spec is wrong. I meant that it had to be adjusted to account for the higher-level types used in the SDK.

I'll take a look at it in the next few days, thanks!

antonengelhardt · 2024-10-14T08:33:56Z

src/traits.rs

+    ///   }
+    /// }
+    ///
+    /// # impl Context for MyRootContext {}


I was wondering if the Context-implementations, which are needed for the successful compilation, should be shown or hidden in the examples. @PiotrSikora

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt · 2024-11-04T13:00:41Z

@PiotrSikora Can you please let the CI checks run so that i can see if it works?

antonengelhardt · 2024-11-04T13:28:51Z

@PiotrSikora Ok, so the Doc Test CI looks fine, but i dont know why the bazel job is failing. Thanks for running the CI ✌🏻

PiotrSikora · 2024-11-04T13:32:49Z

@PiotrSikora Ok, so the Doc Test CI looks fine, but i dont know why the bazel job is failing. Thanks for running the CI ✌🏻

You've added new dependencies, so you need to tell Bazel about them. See DEVELOPMENT.md.

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt · 2024-11-04T14:34:02Z

@PiotrSikora Can you please run them again?

antonengelhardt requested a review from PiotrSikora as a code owner February 27, 2024 16:25

antonengelhardt force-pushed the in-code-documentation branch from ec2927d to d8b90e6 Compare February 27, 2024 16:29

All Examples tested and corrections added

03514d9

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt force-pushed the in-code-documentation branch from 4e79be6 to 03514d9 Compare February 27, 2024 20:04

Merge branch 'master' into in-code-documentation

084d039

PiotrSikora reviewed Apr 30, 2024

View reviewed changes

src/traits.rs Outdated Show resolved Hide resolved

src/traits.rs Show resolved Hide resolved

added missing backticks, tested all scenarios again and added missing…

2d9990a

… stuff Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt added 2 commits April 30, 2024 17:22

update imports in docs & tiny fixes

f190139

Signed-off-by: Anton Engelhardt <[email protected]>

simplify serde annotations

c11460b

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt force-pushed the in-code-documentation branch from 3ea0a4d to c11460b Compare April 30, 2024 16:59

keithmattix mentioned this pull request Jul 31, 2024

Envoy WASM extensions in the present and its future (Proxy-Wasm) envoyproxy/envoy#35420

Open

antonengelhardt and others added 3 commits August 6, 2024 13:10

Merge branch 'main' into in-code-documentation

9d02be2

fix(docs): google cluster name should match example

3c5b187

Signed-off-by: Anton Engelhardt <[email protected]>

fix(docs): format serde plugin config and remove import

41c0119

Signed-off-by: Anton Engelhardt <[email protected]>

PiotrSikora reviewed Aug 13, 2024

View reviewed changes

antonengelhardt added 3 commits August 13, 2024 17:46

fix(docs): log import

0df70c8

Signed-off-by: Anton Engelhardt <[email protected]>

fix(docs): refine docs

9eaac20

Signed-off-by: Anton Engelhardt <[email protected]>

fix(docs): make doc tests compilable with cargo test --doc

4f36bf1

Signed-off-by: Anton Engelhardt <[email protected]>

PiotrSikora reviewed Oct 14, 2024

View reviewed changes

antonengelhardt commented Oct 14, 2024

View reviewed changes

antonengelhardt force-pushed the in-code-documentation branch from 001b7d3 to f151f22 Compare October 14, 2024 13:59

fix(docs): add doc test ci

3b0cf7e

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt force-pushed the in-code-documentation branch from f151f22 to 3b0cf7e Compare October 14, 2024 14:02

antonengelhardt mentioned this pull request Oct 21, 2024

Document hostcalls::dispatch_http_call #201

Closed

docs: documentation for more functions and smaller changes

bf22086

Signed-off-by: Anton Engelhardt <[email protected]>

antonengelhardt force-pushed the in-code-documentation branch 2 times, most recently from c905b28 to bf22086 Compare October 23, 2024 14:15

antonengelhardt changed the title ~~In-Code Documentation~~ docs: functions documented with examples Oct 26, 2024

Merge branch 'main' into in-code-documentation

e325b52

antonengelhardt added 2 commits November 4, 2024 14:49

fix(docs): refine docs

f97217d

Signed-off-by: Anton Engelhardt <[email protected]>

chore(deps): update bazel dependencies

74dc479

Signed-off-by: Anton Engelhardt <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: functions documented with examples #226

docs: functions documented with examples #226

antonengelhardt commented Feb 27, 2024 •

edited

Loading

google-cla bot commented Feb 27, 2024

antonengelhardt commented Feb 27, 2024 •

edited

Loading

antonengelhardt commented Apr 18, 2024

PiotrSikora left a comment

antonengelhardt commented Apr 30, 2024

antonengelhardt commented Apr 30, 2024

antonengelhardt commented May 13, 2024

PiotrSikora Aug 13, 2024

antonengelhardt Oct 13, 2024

PiotrSikora Oct 14, 2024

PiotrSikora commented Aug 13, 2024

PiotrSikora Oct 14, 2024

antonengelhardt Oct 14, 2024

antonengelhardt commented Nov 4, 2024 •

edited

Loading

antonengelhardt commented Nov 4, 2024

PiotrSikora commented Nov 4, 2024

antonengelhardt commented Nov 4, 2024

docs: functions documented with examples #226

Are you sure you want to change the base?

docs: functions documented with examples #226

Conversation

antonengelhardt commented Feb 27, 2024 • edited Loading

Why?

google-cla bot commented Feb 27, 2024

antonengelhardt commented Feb 27, 2024 • edited Loading

antonengelhardt commented Apr 18, 2024

PiotrSikora left a comment

Choose a reason for hiding this comment

antonengelhardt commented Apr 30, 2024

antonengelhardt commented Apr 30, 2024

antonengelhardt commented May 13, 2024

PiotrSikora Aug 13, 2024

Choose a reason for hiding this comment

antonengelhardt Oct 13, 2024

Choose a reason for hiding this comment

PiotrSikora Oct 14, 2024

Choose a reason for hiding this comment

PiotrSikora commented Aug 13, 2024

PiotrSikora Oct 14, 2024

Choose a reason for hiding this comment

antonengelhardt Oct 14, 2024

Choose a reason for hiding this comment

antonengelhardt commented Nov 4, 2024 • edited Loading

antonengelhardt commented Nov 4, 2024

PiotrSikora commented Nov 4, 2024

antonengelhardt commented Nov 4, 2024

antonengelhardt commented Feb 27, 2024 •

edited

Loading

antonengelhardt commented Feb 27, 2024 •

edited

Loading

antonengelhardt commented Nov 4, 2024 •

edited

Loading